Migrate avoid unnecessary set state rule and its tests#238
Conversation
There was a problem hiding this comment.
Code Review
This pull request refactors the AvoidUnnecessarySetStateRule to extend AnalysisRule instead of SolidLintRule, updating its implementation to use the new analyzer APIs. The associated AvoidUnnecessarySetStateVisitor has been updated to extend SimpleAstVisitor and handle diagnostics reporting directly. Additionally, the PR removes an old test file and introduces a comprehensive unit test suite using AnalysisRuleTest. There are no review comments, so no feedback is provided.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
danylo-safonov-solid
left a comment
There was a problem hiding this comment.
A few final touches, otherwise looks good
|
|
||
| /// Unnecessary setState invocations | ||
| Iterable<MethodInvocation> get setStateInvocations => _setStateInvocations; | ||
| final _setStateInvocations = <MethodInvocation>[]; |
There was a problem hiding this comment.
should be before constructor
solid_lints/lib/analysis_options.yaml
Lines 65 to 70 in ad7ba63
| static const lintName = 'avoid_unnecessary_setstate'; | ||
| class AvoidUnnecessarySetStateRule extends AnalysisRule { | ||
| /// The name of the lint rule. | ||
| static const lintName = 'avoid_unnecessary_set_state'; |
There was a problem hiding this comment.
probably best to keep the current name https://lints.solid.software/docs/custom_lints/avoid_unnecessary_setstate
|
|
||
| AvoidUnnecessarySetStateRule._(super.config); | ||
| /// The message shown when the lint rule is triggered. | ||
| static const lintMessage = 'Avoid calling unnecessary setState. ' |
There was a problem hiding this comment.
Doesn't look like there are references to this variable outside of this class, it shouldn't be public. Also, both lintName and lintMessage are already exposed via diagnosticCode so that should be enough for testing
…_state_rule.dart Co-authored-by: danylo-safonov-solid <[email protected]>
danylo-safonov-solid
left a comment
There was a problem hiding this comment.
Is formatting broken in parent branch too? Should we make another PR to fix that (and any other CI checks if necessary)
|
Yes, it looks like formatting was indeed broken on the parent branch. I just ran dart format . locally, and it modified quite a few files across the project. (I have the line length limit set to 80 characters) |
5bcbbf0
into
analysis_server_migration
Migrated the
avoid_unnecessary_set_staterule and its corresponding tests to comply with theanalyzerpackage.Changes
avoid_unnecessary_set_stateto use the current analyzer syntax.